Skip to content

feat(mcp-server): add AsyncHyperpingClient support to create_mcp_server#46

Draft
KhaledSalhab-Develeap wants to merge 2 commits into
feat/5bf946-mcp-serverfrom
feat/48d94e-async-mcp-server
Draft

feat(mcp-server): add AsyncHyperpingClient support to create_mcp_server#46
KhaledSalhab-Develeap wants to merge 2 commits into
feat/5bf946-mcp-serverfrom
feat/48d94e-async-mcp-server

Conversation

@KhaledSalhab-Develeap

Copy link
Copy Markdown
Collaborator

Summary

  • create_mcp_server() now accepts AsyncHyperpingClient and AsyncHyperpingMcpClient in addition to their sync counterparts
  • Each register_*_tools() function uses isinstance dispatch at registration time to define either async def or def tool closures; FastMCP runs coroutine tools natively on the event loop
  • Mixed client types are supported (e.g. async REST client with sync MCP client)

What changed

File Change
src/hyperping/mcp_server/__init__.py Widened client and mcp_client parameter types to include async variants
src/hyperping/mcp_server/_registry.py Widened register_tools signature; added async type imports
src/hyperping/mcp_server/_tools_monitors.py isinstance dispatch: async closures when AsyncHyperpingClient, sync otherwise; same pattern for search_monitors_by_name on mcp_client
src/hyperping/mcp_server/_tools_incidents.py Same isinstance dispatch for 7 incident tools
src/hyperping/mcp_server/_tools_maintenance.py Same isinstance dispatch for 7 maintenance tools
src/hyperping/mcp_server/_tools_outages.py Same isinstance dispatch for 8 outage tools
src/hyperping/mcp_server/_tools_statuspages.py Same isinstance dispatch for 8 status page tools
src/hyperping/mcp_server/_tools_healthchecks.py Same isinstance dispatch for 7 healthcheck tools
src/hyperping/mcp_server/_tools_observability.py isinstance dispatch on mcp_client for 15 observability tools
tests/unit/test_mcp_server/test_factory.py Added TestCreateServerWithAsyncClient (6 tests)
tests/unit/test_mcp_server/test_tools_monitors.py Added TestMonitorToolsAsync (5 tests)
tests/unit/test_mcp_server/test_tools_observability.py Added TestObservabilityToolsAsync (1 test)
tests/unit/test_mcp_server/test_tools_incidents.py Added TestIncidentToolsAsync (1 smoke test)
tests/unit/test_mcp_server/test_tools_maintenance.py Added TestMaintenanceToolsAsync (1 smoke test)
tests/unit/test_mcp_server/test_tools_outages.py Added TestOutageToolsAsync (1 smoke test)
tests/unit/test_mcp_server/test_tools_statuspages.py Added TestStatusPageToolsAsync (1 smoke test)
tests/unit/test_mcp_server/test_tools_healthchecks.py Added TestHealthcheckToolsAsync (1 smoke test)

Design reasoning

Inline isinstance dispatch was chosen over separate async tool files. Each registration function reads cleanly as one function with two branches: the branch selected at server-startup time, not at call time. The approach avoids code duplication across sync and async function bodies (only the await keyword differs), and keeps the public API surface identical regardless of which client type is passed.

MagicMock(spec=AsyncHyperpingClient) satisfies isinstance(mock, AsyncHyperpingClient) via Mock's __class__ property, so no special async-mock plumbing is needed for the dispatch path in tests.

Verification matrix

Check Result
uv run pytest tests/unit/test_mcp_server/ --no-cov -q 128 passed
uv run mypy src/hyperping/mcp_server/ --ignore-missing-imports Success: no issues found
uv run ruff check src/hyperping/mcp_server/ tests/unit/test_mcp_server/ All checks passed
uv run ruff format --check src/ tests/ 0 files need reformatting

Acceptance criteria

  • create_mcp_server(client=AsyncHyperpingClient(...)) returns a valid FastMCP server
  • Tools registered with async client are coroutine functions (asyncio.iscoroutinefunction returns True)
  • Tools registered with sync client remain plain functions
  • Mixed async REST + sync MCP client works: REST tools async, observability tools sync
  • Total tool count (62) unchanged regardless of client type
  • All existing sync tests continue to pass

Add async test fixtures and classes to all 8 MCP server test files,
covering coroutine dispatch for AsyncHyperpingClient and
AsyncHyperpingMcpClient tool registrations.
…er (#48d94e)

Register tools as async coroutines when AsyncHyperpingClient or
AsyncHyperpingMcpClient is passed. Each register_*_tools function
uses isinstance dispatch at registration time so async and sync
clients can be mixed (e.g. async REST + sync MCP). FastMCP runs
coroutine tools natively on the event loop.
@ksalhab89

Copy link
Copy Markdown

Reviewer context (not a merge request):

Extends create_mcp_server so a pre-built AsyncHyperpingClient / AsyncHyperpingMcpClient can back the MCP tools, registering tool functions as coroutines when an async client is detected.

Where to focus review: The isinstance(client, AsyncHyperpingClient) branching in _tools_healthchecks.py, _tools_incidents.py, _tools_maintenance.py (and the other _tools_* modules). Each now has duplicated sync vs async def tool bodies. Confirm both branches stay behaviorally identical (same model_dump() shaping, same optional-field assembly) so the async path cannot drift. Also check _registry.py and __init__.py signature widening to the union types.

Risks / verify: Code duplication is the main maintenance risk; a per-method dispatch helper would be safer. Verify FastMCP actually awaits coroutine-registered tools, and that mcp_client async support is wired through the observability group.

CI status: No checks triggered. This PR targets feat/5bf946-mcp-server, not main; CI (.github/workflows/ci.yml) only runs on PRs to main. Not a failure.

Notes: Stacked on #43. Review after #43 lands. Large duplication but additive and backward-compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants